-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor jitCompile #401
base: master
Are you sure you want to change the base?
Refactor jitCompile #401
Conversation
Hey @nic-donaldson thanks very much, will have a look asap. I guess in the LLVM timeline we're even up to v12 now - do you know if the OrcJIT has changed much from 11 -> 12? |
I don't know! I don't see anything in the release notes, but the LLVM release notes can sometimes be a little incomplete. |
Hey @digego, I don't know much about that stuff, so you're probably better placed to answer it than me. I'm happy to put together a test for the I just tried Any ideas on how best to test the nativecomp stuff cross-platform (and therefore check whether this patchset is safe)? |
Let me get back to you on that. I'll try to take a look at this tomorrow.
…On Wed, May 5, 2021 at 4:51 PM Ben Swift ***@***.***> wrote:
It looks like this code interacts with the extempore-as-dynamic-library
feature, and I have not tested that.
Hey @digego <https://github.com/digego>, I don't know much about that
stuff, so you're probably better placed to answer it than me. I'm happy to
put together a test for the --dll native compile stuff if you can point
me in the right direction of how to test it properly.
I just tried examples/core/native_dll.xtmon my (macOS Big Sur) box and it
got to the clang part---the bitcode and object files were created ok, but
it bombed out because it couldn't find libextempore (I'm not sure where
that's supposed to be).
Any ideas on how best to test the nativecomp stuff cross-platform (and
therefore check whether this patchset is safe)?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#401 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEHPKLSR77JI5ZSVXODZOLTMDTG5ANCNFSM43XXVUVQ>
.
|
btw nic awesome work on all of this, not unappreciated, just super busy at
the moment. my apologies :(
…On Wed, May 5, 2021 at 4:54 PM Andrew Sorensen ***@***.***> wrote:
Let me get back to you on that. I'll try to take a look at this tomorrow.
On Wed, May 5, 2021 at 4:51 PM Ben Swift ***@***.***> wrote:
> It looks like this code interacts with the extempore-as-dynamic-library
> feature, and I have not tested that.
>
> Hey @digego <https://github.com/digego>, I don't know much about that
> stuff, so you're probably better placed to answer it than me. I'm happy to
> put together a test for the --dll native compile stuff if you can point
> me in the right direction of how to test it properly.
>
> I just tried examples/core/native_dll.xtmon my (macOS Big Sur) box and
> it got to the clang part---the bitcode and object files were created ok,
> but it bombed out because it couldn't find libextempore (I'm not sure
> where that's supposed to be).
>
> Any ideas on how best to test the nativecomp stuff cross-platform (and
> therefore check whether this patchset is safe)?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#401 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAEHPKLSR77JI5ZSVXODZOLTMDTG5ANCNFSM43XXVUVQ>
> .
>
|
I've pushed a couple of minor mods today (mostly cmake) to get this all
working on Linux again (haven't tried win yet). There is prob some work to
get going on OSX.
In short it is definitely working on Linux in master. Testing is simply
accomplished by a successful compile.
Note that you do need to build extempore as both exe and dylib. p.s. I
changed define from DYLIB to EXT_DYLIB.
…On Wed, 5 May 2021, 4:51 pm Ben Swift, ***@***.***> wrote:
It looks like this code interacts with the extempore-as-dynamic-library
feature, and I have not tested that.
Hey @digego <https://github.com/digego>, I don't know much about that
stuff, so you're probably better placed to answer it than me. I'm happy to
put together a test for the --dll native compile stuff if you can point
me in the right direction of how to test it properly.
I just tried examples/core/native_dll.xtmon my (macOS Big Sur) box and it
got to the clang part---the bitcode and object files were created ok, but
it bombed out because it couldn't find libextempore (I'm not sure where
that's supposed to be).
Any ideas on how best to test the nativecomp stuff cross-platform (and
therefore check whether this patchset is safe)?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#401 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEHPKLSR77JI5ZSVXODZOLTMDTG5ANCNFSM43XXVUVQ>
.
|
401cb2a
to
96a4353
Compare
Hey @digego I rebased this branch to include those changes. For me adding I noticed a few |
hey @nic-donaldson I'm sorry about the radio silence for so long on this - I'm still keen to see it land (because LLVM 3.8 is waaaay old at this point) and will maybe have some time to look at it soon. The only confounding factor at the moment is that I'm in lockdown with only my macOS laptop, so I'll be relying on GH Actions and the test suite for cross-platform testing (although we don't currently have an end-to-end test for the dylib stuff yet). |
@benswift that's good to hear! I only have a Linux machine I can use at the moment. A while I ago I used a Windows VM on AWS to sort out the LLVM 11 build situation on my branch; it worked but it wasn't fun. |
Yeah, I know what you mean. We can also put the call out the community (e.g. @digego ) to test stuff on Windows. I'll keep you posted - still dealing with lockdowns here (and teaching online) but I'll get to it as soon as I can. |
This PR refactors the jitCompile function!
Please take a look and let me know what you think.
The
jitCompile
function is responsible for a few things:In my opinion it was pretty hard to tell what was going on, especially the dance around setting up the static variables which held different things on the first, second, and third calls to the function. This refactoring mostly removes those variables (sInlineString, sBitcode) and replaces each use with the corresponding file (inline.ll, bitcode.ll, bitcode.ll as bitcode).
There's also a little bit of cleanup, some long lines were split and I indented a few things. As part of this I want to add this
.clang-format
file, which is a pretty standard and opinionated way of formatting C++ code these days. The formatting throughout Extempore's C++ is not very consistent, so I figure this is a good thing to add as many editors these days use the.clang-format
file to decide how to format new or modified code.It looks like this code interacts with the extempore-as-dynamic-library feature, and I have not tested that.
This fits into the LLVM upgrade by making it easier to isolate LLVM from this file.